Skip to content

[codex] Fix baseline Windows OAuth login - #690

Open
brandonkachen wants to merge 6 commits into
mainfrom
codex-fix-baseline-oauth-login
Open

[codex] Fix baseline Windows OAuth login#690
brandonkachen wants to merge 6 commits into
mainfrom
codex-fix-baseline-oauth-login

Conversation

@brandonkachen

@brandonkachen brandonkachen commented May 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes Windows login fragility points that can show up in Bun baseline compiled binaries on older hardware.

  • Bound enhanced hardware fingerprinting to 3 seconds, then fall back to the existing legacy fingerprint path instead of blocking login.
  • Use the native Windows URL handler for browser launch instead of the open package's Windows PowerShell path.
  • Handle asynchronous spawn error events from the Windows URL handler so safeOpen() returns false instead of letting an unhandled child-process error crash the CLI.
  • Reuse the shared withTimeout() helper from common/src/util/promise.ts and add shared coverage for resolve, timeout, and cleanup behavior.
  • Make fingerprint logging/telemetry best-effort so prod-mode CI or pre-login startup cannot turn a valid fingerprint into a login failure.
  • Add a compiled-binary --smoke-login-primitives check and run it in Windows, Linux, and macOS executable smoke paths where the built binary can run on the runner.

Root Cause

The baseline Windows binary can run on older CPUs, but the OAuth flow still depends on local startup/login steps before the browser can complete authentication. Slow or hanging hardware fingerprint commands can prevent the CLI from reaching the login URL or poll loop, Windows URL opening through PowerShell is another brittle dependency from a compiled executable, and spawn() failures are reported through an asynchronous child-process error event rather than the surrounding try/catch.

The first CI smoke also exposed that prod-mode pre-login fingerprint logging/analytics can throw before analytics initialization. That is now guarded so telemetry cannot break login primitives.

Validation

  • bun test cli/src/utils/__tests__/open-url.test.ts cli/src/utils/__tests__/fingerprint.test.ts common/src/util/__tests__/promise.test.ts
  • NEXT_PUBLIC_CB_ENVIRONMENT=prod bun cli/src/index.tsx --smoke-login-primitives
  • NEXT_PUBLIC_CB_ENVIRONMENT=prod ./cli/bin/freebuff --smoke-login-primitives
  • bun run --cwd cli typecheck
  • bun run --cwd common typecheck
  • git diff --check
  • GitHub Actions: build-freebuff passed
  • GitHub Actions: build-and-smoke-freebuff-windows passed

Notes

The workflow smoke test does not perform a real GitHub OAuth browser/account round trip. It validates the compiled executables can load and exercise the local login prerequisites that failed here: timeout behavior, fingerprint generation/classification, and Windows browser-opener command construction.

@jahooma
jahooma marked this pull request as ready for review May 16, 2026 00:30
@codebuff-team

Copy link
Copy Markdown
Contributor

The three actual fixes here are solid and worth porting on their own:

  • common/src/util/promise.ts: withTimeout previously only cleared the timer in the resolve branch, leaking a timer on rejection. The .finally() rewrite is correct and covered by a new test that specifically checks clearTimeout on the rejection path.
  • cli/src/utils/open-url.ts: replacing open's Windows PowerShell path with a direct rundll32.exe url.dll,FileProtocolHandler spawn, plus handling the async error event so safeOpen() returns false instead of an unhandled child-process crash, is a real fix with good test coverage (open-url.test.ts).
  • cli/src/utils/fingerprint.ts: bounding enhanced fingerprinting to 3s and making logging/telemetry best-effort (wrapped so a throw in trackEvent/logger can't fail login) addresses a plausible root cause for hangs on old hardware.

The part that needs work is cli/src/smoke/runtime-primitives.ts and its wiring into cli/src/index.tsx. Two concerns:

  1. smokeLoginNetwork() calls the real production login endpoint (LOGIN_WEBSITE_URL) on every CI run across three platforms, generating live login codes against prod infra just to smoke-test the binary. That's a meaningfully different risk profile than the rest of the PR and deserves its own scrutiny (rate limits, prod data pollution, flakiness).
  2. The --smoke-login-primitives/--smoke-runtime-primitives branches are spliced directly into main() in index.tsx, and the runtime-primitives path reaches across package boundaries into sdk/src/tools/run-terminal-command. That's a lot of production-code surface area and cross-package coupling for a CI-only debug hook.

Suggest splitting: land the withTimeout/fingerprint/open-url fixes as their own small PR, and rework the smoke harness (especially the network call) separately with a maintainer's input on whether it should hit prod at all.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written labels Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants